src/types/WebSocketClient.ts (1)

44-44: Consider making sendBinary required in the interface.

The implementation now exposes sendBinary, so keeping it optional weakens the public contract and forces unnecessary undefined checks for typed consumers.

Suggested type contract tweak

-    sendBinary?(data: string): void;
+    sendBinary(data: string): void;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/types/WebSocketClient.ts` at line 44, The WebSocketClient interface
currently declares sendBinary? as optional; change it to a required method by
removing the optional modifier so it reads sendBinary(data: string): void; then
update any implementers to ensure they provide sendBinary (verify
classes/functions implementing WebSocketClient include a sendBinary method).
Look for the WebSocketClient interface declaration and usages/implementations to
remove unnecessary undefined checks and adjust typings accordingly.

android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt (1)

142-157: Inconsistent exception type caught compared to other methods in this file.

The sendBinaryDataFor method catches URISyntaxException (line 146), but all other methods in this file (e.g., sendDataFor, connectFor, disconnectFor) catch IllegalArgumentException when parsing the URI. While URISyntaxException is technically correct for the URI constructor, this inconsistency could cause confusion.

Consider aligning with the existing pattern for consistency, or updating all methods to use URISyntaxException for correctness.

Option A: Align with existing pattern

     fun sendBinaryDataFor(wsUrl: String, data: String, promise: Promise) {
         val wsUri: URI
         try {
             wsUri = URI(wsUrl)
-        } catch (error: URISyntaxException) {
+        } catch (error: IllegalArgumentException) {
             return promise.reject(error)
         }

Option B: Update all methods to use URISyntaxException (broader refactor)

This would be a larger change affecting sendDataFor, connectFor, disconnectFor, invalidateClientFor, ensureClientFor, and createClientFor.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/src/main/java/com/mattermost/networkclient/WebSocketClientModuleImpl.kt`
around lines 142 - 157, The URI parsing in sendBinaryDataFor currently catches
URISyntaxException which is inconsistent with other methods; change the
exception type to IllegalArgumentException to match sendDataFor, connectFor,
disconnectFor and other methods (or alternatively update all related methods to
URISyntaxException if you prefer that broader refactor), i.e., modify the
try/catch around URI(wsUrl) in sendBinaryDataFor so it catches
IllegalArgumentException and rejects the promise accordingly; ensure references
to clients[wsUri]!!, webSocket!!.send, and promise.reject/resolve remain
unchanged.

android/src/test/java/com/mattermost/networkclient/WebSocketClientModuleImplTest.kt (1)

27-34: Consider using JUnit assertions for better failure messages.

Using Kotlin's assert() provides less informative failure messages compared to JUnit's assertTrue() or AssertJ's assertions. When tests fail, clearer messages help with debugging.

🧪 Proposed improvement using JUnit assertions

+import org.junit.Assert.assertTrue
+
     `@Test`
     fun `sendBinaryDataFor rejects promise for malformed URL`() {
         impl.sendBinaryDataFor("not a valid uri", "dGVzdA==", promise)

         val captor = ArgumentCaptor.forClass(Throwable::class.java)
         verify(promise).reject(captor.capture())
-        assert(captor.value is URISyntaxException)
+        assertTrue(
+            "Expected URISyntaxException but got ${captor.value::class.simpleName}",
+            captor.value is URISyntaxException
+        )
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@android/src/test/java/com/mattermost/networkclient/WebSocketClientModuleImplTest.kt`
around lines 27 - 34, The test method `sendBinaryDataFor rejects promise for
malformed URL` uses Kotlin's `assert()` which gives poor failure messages;
replace that with a JUnit assertion such as `org.junit.Assert.assertTrue` or
`org.junit.Assert.assertThat` to check the captured exception type (use
`assertTrue(captor.value is URISyntaxException)` or a Hamcrest/assertThat
matcher) so failures report clear diagnostics; update the assertion that
currently reads `assert(captor.value is URISyntaxException)` to a JUnit
assertion and ensure the `captor` and `promise` references (from
`impl.sendBinaryDataFor(...)`, `ArgumentCaptor.forClass(Throwable::class.java)`,
and `verify(promise).reject(captor.capture())`) remain unchanged.